Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[18.0][MIG] base_location_geonames_import:Migration to 18.0 #5

Open
wants to merge 56 commits into
base: 18.0
Choose a base branch
from

Conversation

Kimkhoi3010
Copy link
Owner

Depends on: [18.0][MIG] base_location: Migration to 18.0
It found only 1 commit to port:
It was ignored, I added them to blacklist: 8eccd39

Alexis de Lattre and others added 30 commits October 15, 2024 10:05
Add support for states (if states are already present in res.country.state).
Add POT file and FR translation.

Allow to skip entries in the _prepare method.

Insist on the fact that the wizard deletes current better zip entries in the module description and in the wizard itself.
Better error message when the country code inside the file is wrong.

Add a FOR UPDATE NOWAIT at the beginning of the transaction.

Remove France-specific code ; moved to new module l10n_fr_base_location_geonames_import on lp:openerp-french-localization

Add module base_location_geonames_import

Add support for states (if states are already present in res.country.state).
Add POT file and FR translation.

Allow to skip entries in the _prepare method.

Insist on the fact that the wizard deletes current better zip entries in the module description and in the wizard itself.
Better error message when the country code inside the file is wrong.

Add a FOR UPDATE NOWAIT at the beginning of the transaction.

Remove France-specific code ; moved to new module l10n_fr_base_location_geonames_import on lp:openerp-french-localization
…rec_name defined on better.zip.geonames.import

[REF] porting to new api

[IMP] removing 'FOR UPDATE NOWAIT' as ROW EXCLUSIVE lock is already acquired by DELETE and INSERT
http://www.postgresql.org/docs/9.2/static/explicit-locking.html

[REF] select_or_create_state
and tests

[FIX] __openerp__.py PEP8

[FIX] TypeError: unlink() got multiple values for keyword argument 'context'

[IMP] using ir.config_parameter for geonames URL

[FIX] missing cr uid
…ooks

* Added Icon.
* Improve module description and extracted to README.rst.
* Pass country instead of country_id for advance comparisons.
* Allow to transform city name.
* Some code style.
* Do not remove all entries of a country, but only not found.
* Include hooks for transforming some things.
* Include spanish translation.

[FIX] base_location_geonames_import: Use self.env and recordsets
[IMP] base_location_geonames_import: Make tests more efficient

[FIX] base_location_geonames_import: Don't alias Warning

[IMP] base_location_geonames_import: Convert tests to Python + switch imported country

Monaco country is very little and it allows to save some downloaded bytes plus making
a test for entries deletion

[IMP] base_location_geonames_import: Switch author

[FIX] base_location_geonames_import: Fix test

[FIX] base_location_geonames_import: Fix tests

[MIG] Make modules uninstallable

[MIG] Rename manifest files

[FIX] base_location_geonames_import: Test logic changed

For avoiding constant problems with this test, as Monaco data changes a lot,
what we are testing now is the existence of the data, not the exact match of them.
Add option to put city name all upper case
* Tests
* New menu location
* Wizard options fixed

[IMP] base_location_geonames_import

* Speed improvement applying cache for not doing duplicated searches
  over states.
* Tests change to SavepointCase, which only passes over setUp one time.
* [IMP] base_location: Add lat & long to `better.zip`
* Add latitude and longitude columns to `better.zip`

* [IMP] base_location_geonames_import: Add lat/long
* Add support for latitude & longitude to genomes importer
Generated new readme.
Adapted the module due to the refactoring of base_location.
Adapted tests.
Currently translated at 100.0% (23 of 23 strings)

Translation: partner-contact-12.0/partner-contact-12.0-base_location_geonames_import
Translate-URL: https://translation.odoo-community.org/projects/partner-contact-12-0/partner-contact-12-0-base_location_geonames_import/es/
If there are several cities with the same name in different states, previous code doesn't
handle correctly this situation.

We amend this storing in the cached dictionary both city name and state.

Includes a test for checking this specific condition, got from real data in US.

Fixes OCA#749
Currently translated at 100.0% (23 of 23 strings)

Translation: partner-contact-12.0/partner-contact-12.0-base_location_geonames_import
Translate-URL: https://translation.odoo-community.org/projects/partner-contact-12-0/partner-contact-12-0-base_location_geonames_import/pt_BR/
[W0622(redefined-builtin), TestBaseLocationGeonamesImport.test_import_title] Redefining built-in 'zip'
* Performance was very poor due to several reasons:

  * City name translatable.
  * Lack on indexes when deleting obsolete records.
  * Some prefetching.
* City was not being searched also by state, having the chance of returning
  incorrect matches if same city name on different states.
* Previous code was removing all previous records of zip entries!
* Remove possible obsolete FK keys after migrating from v11.
Introducing second variable has provoked an error on that part.
Currently translated at 100.0% (24 of 24 strings)

Translation: partner-contact-13.0/partner-contact-13.0-base_location_geonames_import
Translate-URL: https://translation.odoo-community.org/projects/partner-contact-13-0/partner-contact-13-0-base_location_geonames_import/pt_BR/
In that case, using remove would raise a KeyError.
On the other hand, discard fails silently.
Ivorra78 and others added 12 commits October 15, 2024 10:05
Currently translated at 100.0% (25 of 25 strings)

Translation: partner-contact-16.0/partner-contact-16.0-base_location_geonames_import
Translate-URL: https://translation.odoo-community.org/projects/partner-contact-16-0/partner-contact-16-0-base_location_geonames_import/es/
Currently translated at 96.0% (24 of 25 strings)

Translation: partner-contact-16.0/partner-contact-16.0-base_location_geonames_import
Translate-URL: https://translation.odoo-community.org/projects/partner-contact-16-0/partner-contact-16-0-base_location_geonames_import/it/
Currently translated at 100.0% (25 of 25 strings)

Translation: partner-contact-16.0/partner-contact-16.0-base_location_geonames_import
Translate-URL: https://translation.odoo-community.org/projects/partner-contact-16-0/partner-contact-16-0-base_location_geonames_import/it/
Currently translated at 100.0% (25 of 25 strings)

Translation: partner-contact-16.0/partner-contact-16.0-base_location_geonames_import
Translate-URL: https://translation.odoo-community.org/projects/partner-contact-16-0/partner-contact-16-0-base_location_geonames_import/pt_BR/
- Standard procedure.
- Tests adapted for not blocking external requests.

TT48121
Currently translated at 100.0% (25 of 25 strings)

Translation: partner-contact-17.0/partner-contact-17.0-base_location_geonames_import
Translate-URL: https://translation.odoo-community.org/projects/partner-contact-17-0/partner-contact-17-0-base_location_geonames_import/sv/
Currently translated at 100.0% (25 of 25 strings)

Translation: partner-contact-17.0/partner-contact-17.0-base_location_geonames_import
Translate-URL: https://translation.odoo-community.org/projects/partner-contact-17-0/partner-contact-17-0-base_location_geonames_import/ca/
@Kimkhoi3010 Kimkhoi3010 force-pushed the 18.0-mig-base_location_geonames_import branch from 1ab0433 to a9f127e Compare October 15, 2024 08:51
Copy link

@chaule97 chaule97 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

please add test-requirements.txt


import requests

from odoo import _, api, fields, models

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
from odoo import _, api, fields, models
from odoo import api, fields, models

Comment on lines 108 to 114
raise UserError(
_("Got an error %d when trying to download the file %s.")
% (res_request.status_code, url)
)

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
raise UserError(
_("Got an error %d when trying to download the file %s.")
% (res_request.status_code, url)
)
raise UserError(
self.env._("Got an error %d when trying to download the file %s.", res_request.status_code, url)
)

try:
item.unlink()
except Exception:
logger.info(_("%d could not be deleted %") % item.name)

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
logger.info(_("%d could not be deleted %") % item.name)
logger.info(self.env._("%s could not be deleted %", item.name))

@Kimkhoi3010 Kimkhoi3010 force-pushed the 18.0-mig-base_location_geonames_import branch 2 times, most recently from 9a0fab4 to 393a4a9 Compare October 15, 2024 09:13
)
except Exception:
for item in items:
try:
item.unlink()
except Exception:
logger.info(_("%d could not be deleted %") % item.name)
logger.info(self.env._("%d could not be deleted %", item.name))
Copy link

@chaule97 chaule97 Oct 15, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
logger.info(self.env._("%d could not be deleted %", item.name))
logger.info(self.env._("%s could not be deleted", item.name))

Copy link

@chaule97 chaule97 Oct 15, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

item.name is string, so we replace %d to %s

@Kimkhoi3010 Kimkhoi3010 force-pushed the 18.0-mig-base_location_geonames_import branch 4 times, most recently from 93baf8a to 39788fe Compare October 15, 2024 09:38
@Kimkhoi3010 Kimkhoi3010 force-pushed the 18.0-mig-base_location_geonames_import branch from 39788fe to 6908dd9 Compare October 15, 2024 09:49
@Kimkhoi3010 Kimkhoi3010 force-pushed the 18.0-mig-base_location_geonames_import branch 3 times, most recently from 5308374 to 712137a Compare December 2, 2024 05:16
pedrobaeza and others added 3 commits December 4, 2024 18:35
Befor this commit, there were 2 possible errors re-importing ZIPs:

- If city names are changed on the import process (like capitalization),
  the code for detecting existing entries was not taking that into
  account, failing to find the city.
- If there's more than one found result, using `discard` for removing
  the result from the ZIPs to remove gives an error.

TT52020
@Kimkhoi3010 Kimkhoi3010 force-pushed the 18.0-mig-base_location_geonames_import branch from 712137a to 9d9eac3 Compare December 4, 2024 11:36
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.